Skip to content

Ensure that JSON.GET returns Nil response #2987

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

nic-gibson
Copy link
Contributor

Updated JSONCmd.readReply to return redis.Nil if no results.

Updated JSONCmd.readReply to return redis.Nil if no results
Added a doc line for Val() and Expanded() methods of JSONCmd
Added a test case for non existent keys in json_test.go
@axilis-marko
Copy link

I'm also having issues with this.

@nic-gibson do you know if this will be merged? Should we expect a non error when the key is missing for JSONGet?

@ndyakov
Copy link
Member

ndyakov commented Mar 4, 2025

@nic-gibson can you please resolve conflicts and sync with master ?

@ndyakov
Copy link
Member

ndyakov commented Mar 19, 2025

@nic-gibson can you enable contributions to your fork so we can resolve the conflicts and proceed with this PR.

@axilis-marko
Copy link

Any updates here?

@ndyakov
Copy link
Member

ndyakov commented Apr 24, 2025

@axilis-marko looks like @nic-gibson is not responding. If you would like you are welcomed to open a pr and fix the failing builds

@ndyakov ndyakov self-requested a review April 24, 2025 10:00
Copy link
Member

@ndyakov ndyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update to latest master and fix the failing builds.

ofekshenawa pushed a commit that referenced this pull request Aug 10, 2025
Updated JSONCmd.readReply to return redis.Nil if no results.
Added a doc line for Val() and Expanded() methods of JSONCmd.
Added a test case for non existent keys in json_test.go.

Original-PR: #2987
ofekshenawa pushed a commit that referenced this pull request Aug 11, 2025
Ensure that JSON.GET returns redis.Nil for missing keys/paths,
making it consistent with other Redis commands.

- Restore proper nil detection logic in JSONCmd.readReply
- Add comprehensive test coverage for JSON nil scenarios
- Handle both non-existent keys and non-existent paths consistently
- Distinguish between empty arrays and nil responses
- Add documentation for Val() and Expanded() methods

Original work and problem identification by Nic Gibson.
Enhanced implementation with comprehensive testing and fixes
for the broken nil detection logic.

Fixes #2987
@ofekshenawa
Copy link
Collaborator

Hi @nic-gibson,
I’ve opened a new PR based on your work in this one, since I didn’t have permissions to push changes to your fork.
The new PR includes your contributions along with some adjustments: link.

Thanks again for your work!

ndyakov added a commit that referenced this pull request Aug 12, 2025
* fix conflicts

* Fix JSON nil response handling

Ensure that JSON.GET returns redis.Nil for missing keys/paths,
making it consistent with other Redis commands.

- Restore proper nil detection logic in JSONCmd.readReply
- Add comprehensive test coverage for JSON nil scenarios
- Handle both non-existent keys and non-existent paths consistently
- Distinguish between empty arrays and nil responses
- Add documentation for Val() and Expanded() methods

Original work and problem identification by Nic Gibson.
Enhanced implementation with comprehensive testing and fixes
for the broken nil detection logic.

Fixes #2987

* Fix JSON nil response handling - align with Redis behavior

- Non-existent keys return redis.Nil (consistent with other Redis commands)
- Non-existent paths in existing keys return empty array '[]'
- Fix broken test that was using wrong doc1 reference
- Add comprehensive test coverage for JSON nil scenarios

This aligns with official Redis JSON.GET behavior:
- Missing keys should return nil error like other Redis commands
- Missing paths should return empty JSON array, not error

* Fix JSONDel tests

---------

Co-authored-by: Nic Gibson <[email protected]>
Co-authored-by: Nedyalko Dyakov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants